-
-
Notifications
You must be signed in to change notification settings - Fork 0
Added tests #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added tests #58
Conversation
WalkthroughAdds test infrastructure and fixtures for Elasticsearch, updates a Terraform random_password resource to support conditional creation and stricter password constraints, and introduces a Go test suite plus Atmos/vendor manifests and stack fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Test Runner
participant Suite as ComponentSuite
participant TF as Terraform
participant ES as Elasticsearch Component
participant Verifier as Output Verifier
Runner->>Suite: Run TestRunSuite()
Suite->>TF: terraform init/apply (use fixtures)
TF->>ES: create resources (includes optional random_password)
alt local.create_password == true
TF->>TF: random_password created (numeric,min_lower,min_numeric)
else local.create_password == false
Note right of TF: random_password not created (count = 0)
end
TF->>Suite: outputs (domain, endpoints, IAM, SSM keys)
Suite->>Verifier: validate outputs & drift test
Verifier-->>Suite: pass/fail
Suite-->>Runner: test result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)src/{main,variables,outputs,providers,versions,context}.tf📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.tf📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (11)
test/go.mod (2)
1-3: Use a unique module path and pin toolchain (optional but recommended)“module test” is ambiguous; prefer the repo path. Also consider a toolchain line for reproducibility.
-module test +module github.com/cloudposse-terraform-components/aws-elasticsearch/test go 1.23.0 + +toolchain go1.23.0Ensure CI installs Go 1.23.x (or update toolchain to match your runner).
35-35: Remove unnecessary docdb direct dependencyVerification confirms docdb is not actively imported—only a commented-out reference exists in test/component_test.go. The direct dependency in test/go.mod can be safely removed.
- github.com/aws/aws-sdk-go-v2/service/docdb v1.40.10src/main.tf (1)
79-80: Incorrect SSM parameter descriptionThe description references Aurora Postgres; this parameter stores the Elasticsearch admin password.
- description = "Primary Aurora Postgres Password for the master DB user" + description = "Elasticsearch admin password for the master user"test/component_test.go (5)
27-29: Assert NotEmpty for AWS account IDaccountId is a string; NotNil always passes. Use NotEmpty.
- assert.NotNil(s.T(), accountId) + assert.NotEmpty(s.T(), accountId)
33-34: Handle DeployAtmosComponent errorIgnoring the error can mask real failures.
- options, _ := s.DeployAtmosComponent(s.T(), component, stack, &inputs) - assert.NotNil(s.T(), options) + options, err := s.DeployAtmosComponent(s.T(), component, stack, &inputs) + assert.NoError(s.T(), err) + assert.NotNil(s.T(), options)
39-47: Reduce brittle string expectationsHard-coded prefixes (e.g., “eg-default-ue2-test-e-”) make tests fragile across naming tweaks. Prefer asserting stable ARN/endpoint shapes and substrings.
Example:
- assert.True(s.T(), strings.HasPrefix(domainArn, fmt.Sprintf("arn:aws:es:%s:%s:domain/eg-default-ue2-test-e-", awsRegion, accountId))) + assert.True(s.T(), strings.HasPrefix(domainArn, fmt.Sprintf("arn:aws:es:%s:%s:domain/", awsRegion, accountId))) + assert.Contains(s.T(), domainArn, ":domain/")Apply similar loosening to domainEndpoint/kibanaEndpoint/domainHostname checks.
63-65: Avoid exact match on SSM key; assert prefix/suffixExact equality ties the test to the component name. Prefer prefix/suffix.
- assert.Equal(s.T(), masterPasswordSSMKey, "/elasticsearch/e/password") + assert.True(s.T(), strings.HasPrefix(masterPasswordSSMKey, "/elasticsearch/")) + assert.True(s.T(), strings.HasSuffix(masterPasswordSSMKey, "/password"))
3-16: Drop commented importsRemove commented imports to keep the file clean.
- //"context" - // "github.com/aws/aws-sdk-go-v2/service/docdb" - // awshelper "github.com/cloudposse/test-helpers/pkg/aws"test/fixtures/stacks/catalog/account-map.yaml (1)
23-46: Static backend config looks fine; ensure TEST_ACCOUNT_ID is set in CIThe static backend depends on TEST_ACCOUNT_ID for role resolution. Make sure your pipeline exports it.
test/fixtures/vendor.yaml (1)
7-54: Pin sources to immutable SHAs (optional)Tags can be moved; pinning to commit SHAs improves supply‑chain reproducibility. Keep the tag in a comment for readability.
Example:
source: github.com/cloudposse-terraform-components/aws-dns-delegated.git//src?ref=abcdef1234567890 # v1.535.1test/fixtures/stacks/orgs/default/test/_defaults.yaml (1)
8-9: Consider absolute or environment-aware paths for Terraform state backend.The local backend configuration uses relative paths with template defaults. While this works for test fixtures, relative paths can cause issues when the working directory varies across execution contexts. Ensure all test invocations execute from the expected directory, or document this requirement clearly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
src/main.tf(1 hunks)test/.gitignore(1 hunks)test/component_test.go(1 hunks)test/fixtures/atmos.yaml(1 hunks)test/fixtures/stacks/catalog/account-map.yaml(1 hunks)test/fixtures/stacks/catalog/dns-delegated.yaml(1 hunks)test/fixtures/stacks/catalog/dns-primary.yaml(1 hunks)test/fixtures/stacks/catalog/usecase/basic.yaml(1 hunks)test/fixtures/stacks/catalog/usecase/disabled.yaml(1 hunks)test/fixtures/stacks/catalog/vpc.yaml(1 hunks)test/fixtures/stacks/orgs/default/test/_defaults.yaml(1 hunks)test/fixtures/stacks/orgs/default/test/tests.yaml(1 hunks)test/fixtures/vendor.yaml(1 hunks)test/go.mod(1 hunks)test/run.sh(0 hunks)
💤 Files with no reviewable changes (1)
- test/run.sh
🧰 Additional context used
📓 Path-based instructions (6)
test/fixtures/**
📄 CodeRabbit inference engine (AGENTS.md)
Store Atmos-based test scenarios and supporting data under test/fixtures/
Files:
test/fixtures/stacks/orgs/default/test/tests.yamltest/fixtures/atmos.yamltest/fixtures/stacks/catalog/account-map.yamltest/fixtures/stacks/catalog/vpc.yamltest/fixtures/stacks/catalog/usecase/disabled.yamltest/fixtures/stacks/catalog/dns-primary.yamltest/fixtures/stacks/orgs/default/test/_defaults.yamltest/fixtures/vendor.yamltest/fixtures/stacks/catalog/dns-delegated.yamltest/fixtures/stacks/catalog/usecase/basic.yaml
**/*.{yml,yaml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation for YAML and Markdown files
Files:
test/fixtures/stacks/orgs/default/test/tests.yamltest/fixtures/atmos.yamltest/fixtures/stacks/catalog/account-map.yamltest/fixtures/stacks/catalog/vpc.yamltest/fixtures/stacks/catalog/usecase/disabled.yamltest/fixtures/stacks/catalog/dns-primary.yamltest/fixtures/stacks/orgs/default/test/_defaults.yamltest/fixtures/vendor.yamltest/fixtures/stacks/catalog/dns-delegated.yamltest/fixtures/stacks/catalog/usecase/basic.yaml
test/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Place Go Terratest files under test/ and name them *_test.go
Files:
test/component_test.go
test/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use Go Terratest with github.com/cloudposse/test-helpers and Atmos fixtures for integration tests
Files:
test/component_test.go
src/{main,variables,outputs,providers,versions,context}.tf
📄 CodeRabbit inference engine (AGENTS.md)
Keep the Terraform component source of truth in src/: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf
Files:
src/main.tf
**/*.tf
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tf: Use 2-space indentation for Terraform files
Prefer lower_snake_case for Terraform variables and locals; keep resource/data names descriptive and aligned with Cloud Posse null-label patterns
Ensure Terraform files are formatted (terraform fmt -recursive) and contain no formatting violations
Comply with TFLint rules configured in .tflint.hcl; do not commit lint violations
Files:
src/main.tf
🪛 OSV Scanner (2.2.3)
test/go.mod
[HIGH] 1-1: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 1-1: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (14)
test/.gitignore (1)
1-5: LGTM!All entries appropriately exclude test artifacts and Atmos-generated metadata.
test/fixtures/atmos.yaml (1)
1-77: LGTM!Configuration is comprehensive, well-documented, and appropriate for test infrastructure. Atmos settings, logging, and template configurations are correctly configured.
test/fixtures/stacks/catalog/vpc.yaml (1)
1-19: LGTM!Test VPC fixture is well-configured for test scenarios with appropriate defaults (public subnets enabled, NAT disabled, flow logs disabled).
test/fixtures/stacks/catalog/dns-primary.yaml (1)
1-9: LGTM!DNS primary fixture is appropriately configured with test domain and minimal configuration.
test/fixtures/stacks/catalog/dns-delegated.yaml (1)
1-10: LGTM!DNS delegated fixture properly complements the DNS primary configuration for comprehensive DNS testing.
test/fixtures/stacks/catalog/usecase/basic.yaml (1)
1-18: LGTM!Elasticsearch basic fixture provides comprehensive configuration for an enabled instance. Note: metadata.component is set to "target"—verify this is the intended generic/placeholder component reference.
test/fixtures/stacks/catalog/usecase/disabled.yaml (1)
1-17: LGTM!Disabled Elasticsearch fixture correctly mirrors the basic configuration with enabled: false, providing test coverage for the absent Elasticsearch scenario.
test/fixtures/stacks/orgs/default/test/tests.yaml (1)
1-7: LGTM!Test stack imports are properly organized with defaults first, followed by shared infrastructure (DNS, VPC), and test scenarios. Ensure orgs/default/test/_defaults.yaml is properly configured and available.
src/main.tf (3)
60-60: Conditional password creation LGTMcount = local.create_password ? 1 : 0 prevents creating a secret when one is supplied. Good.
64-67: Attribute rename and charset flags LGTM; consider restricting special charactersnumeric=true fixes the provider attribute. If OpenSearch rejects certain symbols, set override_special to an allowed set to avoid failures during domain creation.
Would you like me to propose an override_special set aligned with AWS constraints?
69-73: Minimum character class constraints LGTMmin_lower/min_numeric ensure complexity. No issues.
test/fixtures/stacks/orgs/default/test/_defaults.yaml (3)
44-50: Verify contradictory EKS configuration.The account has
eks: true(line 44) but the tags containeks: false(line 50). This appears contradictory—confirm whether both are intended or if one should be corrected.
66-66: Verify empty string for terraform_roles is intentional.The
terraform_roles.default-testis set to an empty string. Confirm this is the expected test configuration; if a non-empty role is needed, update accordingly.
1-34: YAML structure and indentation look good.The import statement, backend configuration, and terraform variables follow correct 2-space indentation and are well-structured for a test fixture.
|
/terratest |
1 similar comment
|
/terratest |
|
These changes were released in v1.536.0. |
what
why
Summary by CodeRabbit
New Features
Chores